From: Jonas Dreßler Date: Thu, 24 Jul 2025 16:51:25 +0000 (+0200) Subject: st/widget: Don't animate theme node updates on theme context changes X-Git-Tag: archive/raspbian/50.5-1+rpi1~1^2~88^2~9^2^2~11 X-Git-Url: https://dgit.raspbian.org/%22http://www.example.com/cgi/%22/%22http:/www.example.com/cgi/%22?a=commitdiff_plain;h=1324869f485295d5f051282fd70f0b996949c2e6;p=gnome-shell.git st/widget: Don't animate theme node updates on theme context changes With commit 4dfa39457e, I was assuming that emitting `::changed` on the StThemeContext, triggering StWidgets recursive style-changed machinery, is enough to get rid of all the theme nodes that are in use, ensuring they're never accessed again after the `::changed` emission. That assumption turned out to be wrong: `st_widget_recompute_style()` animates updates of theme nodes using StThemeNodeTransition, and StThemeNodeTransition keeps the old theme node around, accessing it at various later points. This caused various crashes on extension loading/unloading, because that triggers a theme context change, where a StThemeNodeTransition would later access an outdated theme node, pointing to CSS properties that are already freed. That said, reverting 4dfa39457e doesn't seem like the best idea, because other calls to `st_theme_context_changed()` also seem to make the same (incorrect) assumption, eg. `st_theme_context_set_theme()` seems like a good candidate for a similar crash. Probably St just shouldn't try to animate on theme context changes: The old theme nodes are no longer usable after those, but StThemeNodeTransition expects them to be, so that's just a bad idea. So change the style-changed machinery a bit to allow disabling transitions and use that to avoid transitions on theme context changes. Fixes: 4dfa39457e ("st/theme-context: Also remove theme nodes on stylesheet changes") Closes: https://gitlab.gnome.org/GNOME/gnome-shell/-/issues/7339 Part-of: (cherry picked from commit 9ed77ef13e809da12567a5d6725ac2525581f28b) --- diff --git a/src/st/st-widget.c b/src/st/st-widget.c index 0b727ac47..3d9b76ebb 100644 --- a/src/st/st-widget.c +++ b/src/st/st-widget.c @@ -126,8 +126,15 @@ static guint signals[LAST_SIGNAL] = { 0, }; G_DEFINE_TYPE_WITH_PRIVATE (StWidget, st_widget, CLUTTER_TYPE_ACTOR); #define ST_WIDGET_PRIVATE(w) ((StWidgetPrivate *)st_widget_get_instance_private (w)) -static void st_widget_recompute_style (StWidget *widget, - StThemeNode *old_theme_node); +typedef enum { + STYLE_CHANGE_FLAGS_NONE = 0, + STYLE_CHANGE_FLAGS_NO_TRANSITIONS = 1 << 0, +} StyleChangeFlags; + +static void st_widget_recompute_style (StWidget *widget, + StThemeNode *old_theme_node, + StyleChangeFlags flags); + static gboolean st_widget_real_navigate_focus (StWidget *widget, ClutterActor *from, StDirectionType direction); @@ -482,7 +489,12 @@ st_widget_unmap (ClutterActor *actor) } static void -notify_children_of_style_change (ClutterActor *self) +st_widget_style_changed_internal (StWidget *widget, + StyleChangeFlags flags); + +static void +notify_children_of_style_change (ClutterActor *self, + StyleChangeFlags flags) { ClutterActorIter iter; ClutterActor *actor; @@ -491,9 +503,9 @@ notify_children_of_style_change (ClutterActor *self) while (clutter_actor_iter_next (&iter, &actor)) { if (ST_IS_WIDGET (actor)) - st_widget_style_changed (ST_WIDGET (actor)); + st_widget_style_changed_internal (ST_WIDGET (actor), flags); else - notify_children_of_style_change (actor); + notify_children_of_style_change (actor, flags); } } @@ -503,8 +515,9 @@ st_widget_real_style_changed (StWidget *self) clutter_actor_queue_redraw ((ClutterActor *) self); } -void -st_widget_style_changed (StWidget *widget) +static void +st_widget_style_changed_internal (StWidget *widget, + StyleChangeFlags flags) { StWidgetPrivate *priv = st_widget_get_instance_private (widget); StThemeNode *old_theme_node = NULL; @@ -518,22 +531,28 @@ st_widget_style_changed (StWidget *widget) /* update the style only if we are mapped */ if (clutter_actor_is_mapped (CLUTTER_ACTOR (widget))) - st_widget_recompute_style (widget, old_theme_node); + st_widget_recompute_style (widget, old_theme_node, flags); /* Descend through all children. If the actor is not mapped, * children will clear their theme node without recomputing style. */ - notify_children_of_style_change (CLUTTER_ACTOR (widget)); + notify_children_of_style_change (CLUTTER_ACTOR (widget), flags); if (old_theme_node) g_object_unref (old_theme_node); } +void +st_widget_style_changed (StWidget *widget) +{ + st_widget_style_changed_internal (widget, STYLE_CHANGE_FLAGS_NONE); +} + static void on_theme_context_changed (StThemeContext *context, ClutterStage *stage) { - notify_children_of_style_change (CLUTTER_ACTOR (stage)); + notify_children_of_style_change (CLUTTER_ACTOR (stage), STYLE_CHANGE_FLAGS_NO_TRANSITIONS); } static StThemeNode * @@ -1632,8 +1651,9 @@ on_transition_completed (StThemeNodeTransition *transition, } static void -st_widget_recompute_style (StWidget *widget, - StThemeNode *old_theme_node) +st_widget_recompute_style (StWidget *widget, + StThemeNode *old_theme_node, + StyleChangeFlags flags) { StWidgetPrivate *priv = st_widget_get_instance_private (widget); StThemeNode *new_theme_node = st_widget_get_theme_node (widget); @@ -1655,7 +1675,9 @@ st_widget_recompute_style (StWidget *widget, if (!geometry_equal) clutter_actor_queue_relayout ((ClutterActor *) widget); - transition_duration = st_theme_node_get_transition_duration (new_theme_node); + transition_duration = (flags & STYLE_CHANGE_FLAGS_NO_TRANSITIONS) != 0 + ? 0 + : st_theme_node_get_transition_duration (new_theme_node); paint_equal = st_theme_node_paint_equal (old_theme_node, new_theme_node); @@ -1730,8 +1752,8 @@ st_widget_ensure_style (StWidget *widget) if (priv->is_style_dirty) { - st_widget_recompute_style (widget, NULL); - notify_children_of_style_change (CLUTTER_ACTOR (widget)); + st_widget_recompute_style (widget, NULL, STYLE_CHANGE_FLAGS_NONE); + notify_children_of_style_change (CLUTTER_ACTOR (widget), STYLE_CHANGE_FLAGS_NONE); } }